p2m: Always check to see if removing a p2m entry actually worked
authorGeorge Dunlap <george.dunlap@citrix.com>
Tue, 28 Nov 2017 12:13:03 +0000 (13:13 +0100)
committerJan Beulich <jbeulich@suse.com>
Tue, 28 Nov 2017 12:13:03 +0000 (13:13 +0100)
commit92790672dedf2eab042e04ecc277c19d40fd348a
treedd8498e9fe153157e2f1945d161ae57f664c322d
parenta1c6c6768971ea387d7eba0803908ef0928b43ac
p2m: Always check to see if removing a p2m entry actually worked

The PoD zero-check functions speculatively remove memory from the p2m,
then check to see if it's completely zeroed, before putting it in the
cache.

Unfortunately, the p2m_set_entry() calls may fail if the underlying
pagetable structure needs to change and the domain has exhausted its
p2m memory pool: for instance, if we're removing a 2MiB region out of
a 1GiB entry (in the p2m_pod_zero_check_superpage() case), or a 4k
region out of a 2MiB or larger entry (in the p2m_pod_zero_check()
case); and the return value is not checked.

The underlying mfn will then be added into the PoD cache, and at some
point mapped into another location in the p2m.  If the guest
afterwards ballons out this memory, it will be freed to the hypervisor
and potentially reused by another domain, in spite of the fact that
the original domain still has writable mappings to it.

There are several places where p2m_set_entry() shouldn't be able to
fail, as it is guaranteed to write an entry of the same order that
succeeded before.  Add a backstop of crashing the domain just in case,
and an ASSERT_UNREACHABLE() to flag up the broken assumption on debug
builds.

While we're here, use PAGE_ORDER_2M rather than a magic constant.

This is part of XSA-247.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
xen/arch/x86/mm/p2m-pod.c